Conversation
📝 WalkthroughWalkthrough此PR实现了服务器重启功能,包括后端优雅关闭与重启逻辑、HTTP管理端点处理,以及前端Web界面中的重启菜单项与本地化字符串支持。 Changes
Sequence DiagramsequenceDiagram
participant User as 用户
participant UI as 前端UI<br/>(nav-user.tsx)
participant Transport as HTTP传输层<br/>(HttpTransport)
participant Admin as 管理处理器<br/>(AdminHandler)
participant Server as HTTP服务器<br/>(main.go)
User->>UI: 点击"重启服务器"
UI->>UI: 显示确认对话框
User->>UI: 确认重启
UI->>Transport: restartServer()
Transport->>Admin: POST /restart
Admin->>Admin: handleRestart验证POST方法
Admin->>Server: 调用restartFn()
Admin-->>Transport: 202 Accepted
Transport-->>UI: Promise resolved
UI-->>User: 显示重启中状态
Server->>Server: 执行shutdownServer()
Server->>Server: 等待活跃请求完成
Server->>Server: 关闭pprof/OAuth/HTTP服务器
Server->>Server: re-exec二进制文件
Server->>Server: 退出当前进程
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
web/src/pages/projects/index.tsx (1)
44-60: 排序实现正确,有小幅优化空间。当前实现正确使用
slice()避免原数组变异,Number.isFinite检查也妥善处理了无效日期。不过在比较函数中重复创建Date对象可以优化:♻️ 可选优化:预先计算时间戳
const sortedProjects = useMemo(() => { if (!projects) { return undefined; } - return projects.slice().sort((a, b) => { - const timeA = Number.isFinite(new Date(a.createdAt).getTime()) - ? new Date(a.createdAt).getTime() - : 0; - const timeB = Number.isFinite(new Date(b.createdAt).getTime()) - ? new Date(b.createdAt).getTime() - : 0; - if (timeA !== timeB) { - return timeA - timeB; - } - return a.id - b.id; - }); + return projects + .map((p) => { + const time = new Date(p.createdAt).getTime(); + return { ...p, _sortTime: Number.isFinite(time) ? time : 0 }; + }) + .sort((a, b) => { + if (a._sortTime !== b._sortTime) { + return a._sortTime - b._sortTime; + } + return a.id - b.id; + }); }, [projects]);对于常规项目列表规模,当前实现性能影响可忽略不计。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/projects/index.tsx` around lines 44 - 60, The sort comparator recreates Date objects twice per item; optimize by precomputing numeric timestamps before sorting: inside the useMemo that defines sortedProjects, map projects to temporary objects (or use a memoized map) that attach a parsed timestamp for each item's createdAt (e.g., computedTimestamp), then sort using those computed timestamps and fall back to id when equal; update references to timeA/timeB to use the precomputed timestamp field so Date() is not constructed repeatedly in the comparator.web/src/locales/zh.json (1)
1085-1087: 文件末尾存在多余空行。建议移除多余的空行,保持 JSON 文件格式一致性。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/locales/zh.json` around lines 1085 - 1087, 文件 web/src/locales/zh.json 在末尾包含多余空行:打开 zh.json(注意检查文件尾部),删除多余的空白/空行以确保文件末尾不包含空行并保持有效的 JSON 语法,保存时确保只保留一个必要的换行或无多余空白字符以符合项目格式约定。cmd/maxx/main.go (1)
472-475: 不可达代码
os.Exit(0)之后的return nil永远不会执行。虽然不影响功能,但属于死代码。♻️ 建议移除
log.Printf("[Admin] Started new process (pid=%d). Exiting current process.", cmd.Process.Pid) os.Exit(0) - return nil + return nil // unreachable, but satisfies return type }或者添加注释说明这是为了满足返回类型要求。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/maxx/main.go` around lines 472 - 475, The line "return nil" after os.Exit(0) in the main.go path is unreachable dead code; remove the unreachable return or, if you prefer to keep it for readability/compile reasons, replace it with a clear comment explaining it's intentionally unreachable (e.g., after the os.Exit call in the block that logs "[Admin] Started new process (pid=%d). Exiting current process."); update the code around cmd.Process.Pid and os.Exit to either omit the return or add the explanatory comment so no dead code remains.web/src/components/layout/app-sidebar/nav-user.tsx (3)
226-232: 不必要的空 React Fragment
<>...</>在这里作为DropdownMenuContent的直接子元素是多余的,可以直接放置DropdownMenuSeparator和DropdownMenuItem。♻️ 建议简化
</DropdownMenuGroup> - <> - <DropdownMenuSeparator /> - <DropdownMenuItem onClick={handleRestartServer}> - <RefreshCw /> - <span>{t('nav.restartServer')}</span> - </DropdownMenuItem> - </> + <DropdownMenuSeparator /> + <DropdownMenuItem onClick={handleRestartServer}> + <RefreshCw /> + <span>{t('nav.restartServer')}</span> + </DropdownMenuItem> </DropdownMenuContent>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/app-sidebar/nav-user.tsx` around lines 226 - 232, 当前在 DropdownMenuContent 的直接子节点中使用了多余的空 React Fragment (<>...</>); 请移除该片段并将 DropdownMenuSeparator 和 DropdownMenuItem (带 onClick={handleRestartServer} 和 RefreshCw 图标及 t('nav.restartServer') 文本) 直接作为子元素放置,确保不改变现有 props 或事件处理器(例如 handleRestartServer)的绑定。
42-45: 桌面端重启检测逻辑重复类型断言
desktopRestartAvailable变量的类型断言在使用时又重复了一次(Lines 55-57)。建议提取为一个复用的类型定义或工具函数,以减少代码重复和潜在的类型不一致风险。♻️ 建议优化
+type DesktopLauncher = { + go?: { desktop?: { LauncherApp?: { RestartServer?: () => Promise<void> } } }; +}; + +const getDesktopLauncher = () => + (window as unknown as DesktopLauncher).go?.desktop?.LauncherApp; + const desktopRestartAvailable = - typeof window !== 'undefined' && - !!(window as unknown as { go?: { desktop?: { LauncherApp?: { RestartServer?: () => unknown } } } }) - .go?.desktop?.LauncherApp?.RestartServer; + typeof window !== 'undefined' && !!getDesktopLauncher()?.RestartServer; const handleRestartServer = async () => { if (!window.confirm(t('nav.restartServerConfirm'))) return; try { if (desktopRestartAvailable) { - const launcher = (window as unknown as { - go?: { desktop?: { LauncherApp?: { RestartServer?: () => Promise<void> } } }; - }).go?.desktop?.LauncherApp; + const launcher = getDesktopLauncher(); if (!launcher?.RestartServer) { throw new Error('Desktop restart is unavailable.'); }Also applies to: 54-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/app-sidebar/nav-user.tsx` around lines 42 - 45, The repeated complex type assertion around window.go.desktop.LauncherApp.RestartServer (used when computing desktopRestartAvailable and again at lines 55-57) should be replaced by a single reusable type or helper to avoid duplication; create a narrow type alias or utility function (e.g., isDesktopRestartAvailable or a WindowWithLauncherApp type) that encapsulates the cast and existence check for RestartServer and use that helper wherever desktop restart availability is tested (references: desktopRestartAvailable, RestartServer, go.desktop.LauncherApp).
65-70: 冗余的typeof window检查在
'use client'组件中,window对象始终存在,Line 67 的检查是多余的。♻️ 建议简化
} catch (error) { console.error('Restart server failed:', error); - if (typeof window !== 'undefined') { - window.alert(t('nav.restartServerFailed')); - } + window.alert(t('nav.restartServerFailed')); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/app-sidebar/nav-user.tsx` around lines 65 - 70, The catch block in nav-user.tsx redundantly checks typeof window inside this 'use client' component; remove the unnecessary guard and call window.alert directly (inside the catch where console.error('Restart server failed:', error) is called) so the code uses window.alert(t('nav.restartServerFailed')) without the typeof window check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/maxx/main.go`:
- Around line 451-475: The restartServer function currently calls shutdownServer
before attempting to start the replacement process, so if
exec.Command(...).Start() fails the server is left down; change the flow in
restartServer (and related usage of restartInProgress) to first validate and
start the replacement process and only call shutdownServer("restart") after
cmd.Start() succeeds: locate restartServer, ensure os.Executable() is checked
early, create exec.Command with Stdout/Stderr/Env, call cmd.Start() and on
success log the new pid then call shutdownServer and exit; if cmd.Start()
returns an error, reset the atomic restartInProgress flag and return the error
so the existing process stays up. Make sure to reference restartInProgress,
restartServer, shutdownServer, and cmd.Start in your change.
---
Nitpick comments:
In `@cmd/maxx/main.go`:
- Around line 472-475: The line "return nil" after os.Exit(0) in the main.go
path is unreachable dead code; remove the unreachable return or, if you prefer
to keep it for readability/compile reasons, replace it with a clear comment
explaining it's intentionally unreachable (e.g., after the os.Exit call in the
block that logs "[Admin] Started new process (pid=%d). Exiting current
process."); update the code around cmd.Process.Pid and os.Exit to either omit
the return or add the explanatory comment so no dead code remains.
In `@web/src/components/layout/app-sidebar/nav-user.tsx`:
- Around line 226-232: 当前在 DropdownMenuContent 的直接子节点中使用了多余的空 React Fragment
(<>...</>); 请移除该片段并将 DropdownMenuSeparator 和 DropdownMenuItem (带
onClick={handleRestartServer} 和 RefreshCw 图标及 t('nav.restartServer') 文本)
直接作为子元素放置,确保不改变现有 props 或事件处理器(例如 handleRestartServer)的绑定。
- Around line 42-45: The repeated complex type assertion around
window.go.desktop.LauncherApp.RestartServer (used when computing
desktopRestartAvailable and again at lines 55-57) should be replaced by a single
reusable type or helper to avoid duplication; create a narrow type alias or
utility function (e.g., isDesktopRestartAvailable or a WindowWithLauncherApp
type) that encapsulates the cast and existence check for RestartServer and use
that helper wherever desktop restart availability is tested (references:
desktopRestartAvailable, RestartServer, go.desktop.LauncherApp).
- Around line 65-70: The catch block in nav-user.tsx redundantly checks typeof
window inside this 'use client' component; remove the unnecessary guard and call
window.alert directly (inside the catch where console.error('Restart server
failed:', error) is called) so the code uses
window.alert(t('nav.restartServerFailed')) without the typeof window check.
In `@web/src/locales/zh.json`:
- Around line 1085-1087: 文件 web/src/locales/zh.json 在末尾包含多余空行:打开
zh.json(注意检查文件尾部),删除多余的空白/空行以确保文件末尾不包含空行并保持有效的 JSON
语法,保存时确保只保留一个必要的换行或无多余空白字符以符合项目格式约定。
In `@web/src/pages/projects/index.tsx`:
- Around line 44-60: The sort comparator recreates Date objects twice per item;
optimize by precomputing numeric timestamps before sorting: inside the useMemo
that defines sortedProjects, map projects to temporary objects (or use a
memoized map) that attach a parsed timestamp for each item's createdAt (e.g.,
computedTimestamp), then sort using those computed timestamps and fall back to
id when equal; update references to timeA/timeB to use the precomputed timestamp
field so Date() is not constructed repeatedly in the comparator.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
cmd/maxx/main.gointernal/desktop/launcher.gointernal/handler/admin.goweb/src/components/layout/app-sidebar/nav-user.tsxweb/src/components/ui/chart.tsxweb/src/hooks/queries/use-usage-stats.tsweb/src/lib/transport/http-transport.tsweb/src/lib/transport/interface.tsweb/src/locales/en.jsonweb/src/locales/zh.jsonweb/src/pages/projects/index.tsxweb/src/pages/stats/index.tsx
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (3)
internal/desktop/launcher.go (1)
internal/handler/admin.go (1)
AdminHandler(20-25)
cmd/maxx/main.go (1)
internal/core/server.go (2)
GracefulShutdownTimeout(16-16)HTTPShutdownTimeout(18-18)
web/src/components/layout/app-sidebar/nav-user.tsx (4)
web/src/components/theme-provider.tsx (1)
useTheme(138-144)launcher/script.js (1)
launcher(22-31)web/src/components/ui/dropdown-menu.tsx (4)
DropdownMenuSeparator(239-239)DropdownMenuItem(235-235)DropdownMenuContent(232-232)DropdownMenu(229-229)web/src/components/ui/sidebar.tsx (2)
SidebarMenuItem(688-688)SidebarMenu(684-684)
🔇 Additional comments (12)
web/src/hooks/queries/use-usage-stats.ts (1)
6-6: LGTM!正确使用了 TanStack Query v5 的
keepPreviousData作为placeholderData,在数据重新加载时保持 UI 稳定性,提升用户体验。Also applies to: 120-120
web/src/components/ui/chart.tsx (1)
66-68: LGTM!通过显式设置
width="100%"、height="100%"和minHeight={1},确保图表在容器内正确响应式渲染,避免父容器高度未设置时图表塌陷。web/src/lib/transport/interface.ts (1)
120-122: LGTM!新增的
restartServer方法遵循现有接口设计模式,返回类型Promise<void>适合重启操作的语义。web/src/locales/zh.json (1)
86-88: LGTM!中文翻译文本准确自然,与英文版本保持一致。
internal/desktop/launcher.go (1)
209-212: LGTM!正确地将桌面端重启能力绑定到 AdminHandler,空值检查也是合理的防御性编程。
web/src/lib/transport/http-transport.ts (1)
326-330: LGTM!
restartServer方法实现正确,使用this.client确保请求携带认证信息,与 Transport 接口签名一致。web/src/pages/stats/index.tsx (1)
901-902: LGTM!使用固定高度(400px)替代百分比高度是 Recharts 的常见最佳实践,可以避免动态父容器高度导致的渲染问题。外层 div 的
h-[400px]与ResponsiveContainer的height={400}配合使用确保图表尺寸稳定。web/src/pages/projects/index.tsx (1)
110-112: LGTM!正确使用
sortedProjects替代原始projects进行渲染,确保列表按创建时间排序显示。cmd/maxx/main.go (1)
406-449: 优雅关闭实现良好
shutdownServer函数实现了完整的优雅关闭流程:
- 等待活跃请求完成
- 停止后台任务
- 停止 pprof 和 OAuth 服务
- 关闭 HTTP 服务器
使用了
core.GracefulShutdownTimeout和core.HTTPShutdownTimeout常量,逻辑清晰且具有良好的错误处理。web/src/locales/en.json (1)
86-88: 本地化字符串添加合理新增的三个本地化键值清晰且信息完整:
- 重启确认提示适当地警告了可能的连接中断
- 失败消息提供了可操作的指导
与现有的本地化结构保持一致。
internal/handler/admin.go (2)
110-129: 重启端点实现正确
handleRestart实现良好:
- 正确强制 POST 方法并设置 Allow 头
restartFn == nil时返回 501 Not Implemented,防御性编程- 在 goroutine 中执行重启,确保 HTTP 响应(202 Accepted)能够在服务器关闭前发送
- 使用 202 Accepted 状态码符合异步操作的 HTTP 语义
该端点位于
/api/admin/路径下,受authMiddleware保护,需要认证才能访问。
24-24: 回调注入模式设计合理使用
SetRestartFunc注入重启回调而非构造函数参数,适合这种可选功能(桌面端和纯 Web 部署场景可能不同)。这种设计保持了AdminHandler的灵活性。Also applies to: 36-39
| restartServer := func() error { | ||
| if !atomic.CompareAndSwapInt32(&restartInProgress, 0, 1) { | ||
| return fmt.Errorf("restart already in progress") | ||
| } | ||
|
|
||
| shutdownServer("restart") | ||
|
|
||
| executable, err := os.Executable() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to locate executable: %w", err) | ||
| } | ||
|
|
||
| cmd := exec.Command(executable, os.Args[1:]...) | ||
| cmd.Stdout = os.Stdout | ||
| cmd.Stderr = os.Stderr | ||
| cmd.Env = os.Environ() | ||
|
|
||
| if err := cmd.Start(); err != nil { | ||
| return fmt.Errorf("failed to start new process: %w", err) | ||
| } | ||
|
|
||
| log.Printf("[Admin] Started new process (pid=%d). Exiting current process.", cmd.Process.Pid) | ||
| os.Exit(0) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
重启失败时服务器无法恢复的严重问题
restartServer 函数在调用 shutdownServer 之后(Line 456)才尝试启动新进程。如果 cmd.Start() 失败(Line 468),旧进程已经关闭,服务器将处于不可用状态且无法自动恢复。
建议在关闭服务器之前先验证可执行文件是否可访问,或者调整顺序以降低风险。
🛠️ 建议的修复方案
restartServer := func() error {
if !atomic.CompareAndSwapInt32(&restartInProgress, 0, 1) {
return fmt.Errorf("restart already in progress")
}
- shutdownServer("restart")
-
executable, err := os.Executable()
if err != nil {
+ atomic.StoreInt32(&restartInProgress, 0)
return fmt.Errorf("failed to locate executable: %w", err)
}
+ // Verify executable is accessible before shutting down
+ if _, err := os.Stat(executable); err != nil {
+ atomic.StoreInt32(&restartInProgress, 0)
+ return fmt.Errorf("executable not accessible: %w", err)
+ }
+
+ shutdownServer("restart")
+
cmd := exec.Command(executable, os.Args[1:]...)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.Env = os.Environ()
if err := cmd.Start(); err != nil {
+ // At this point shutdown already happened, log critical error
+ log.Printf("[Admin] CRITICAL: Failed to start new process after shutdown: %v", err)
return fmt.Errorf("failed to start new process: %w", err)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/maxx/main.go` around lines 451 - 475, The restartServer function
currently calls shutdownServer before attempting to start the replacement
process, so if exec.Command(...).Start() fails the server is left down; change
the flow in restartServer (and related usage of restartInProgress) to first
validate and start the replacement process and only call
shutdownServer("restart") after cmd.Start() succeeds: locate restartServer,
ensure os.Executable() is checked early, create exec.Command with
Stdout/Stderr/Env, call cmd.Start() and on success log the new pid then call
shutdownServer and exit; if cmd.Start() returns an error, reset the atomic
restartInProgress flag and return the error so the existing process stays up.
Make sure to reference restartInProgress, restartServer, shutdownServer, and
cmd.Start in your change.
7662a5f to
2b7964a
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cmd/maxx/main.go (1)
451-475:⚠️ Potential issue | 🟠 Major重启失败时服务器无法恢复的问题仍然存在
当前代码在调用
shutdownServer之后才尝试获取可执行文件路径和启动新进程。如果os.Executable()或cmd.Start()失败,服务器已经关闭且无法恢复。建议在关闭服务器之前先验证可执行文件是否可访问。
🛠️ 建议的修复方案
restartServer := func() error { if !atomic.CompareAndSwapInt32(&restartInProgress, 0, 1) { return fmt.Errorf("restart already in progress") } - shutdownServer("restart") - executable, err := os.Executable() if err != nil { + atomic.StoreInt32(&restartInProgress, 0) return fmt.Errorf("failed to locate executable: %w", err) } + // 在关闭前验证可执行文件可访问 + if _, err := os.Stat(executable); err != nil { + atomic.StoreInt32(&restartInProgress, 0) + return fmt.Errorf("executable not accessible: %w", err) + } + + shutdownServer("restart") + cmd := exec.Command(executable, os.Args[1:]...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr cmd.Env = os.Environ() if err := cmd.Start(); err != nil { + // 此时 shutdown 已执行,记录严重错误 + log.Printf("[Admin] CRITICAL: Failed to start new process after shutdown: %v", err) return fmt.Errorf("failed to start new process: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/maxx/main.go` around lines 451 - 475, The restartServer closure currently calls shutdownServer("restart") before confirming that os.Executable() and starting the new process (exec.Command + cmd.Start()) succeed; if those fail the server is down with restartInProgress set. Change restartServer so it first obtains and validates the executable path (os.Executable), constructs the exec.Command (os.Args[1:], os.Environ()), and attempts to start the new process (cmd.Start()) — only after cmd.Start() succeeds should shutdownServer("restart") be invoked and os.Exit called; on any failure, ensure restartInProgress is reset (atomic.StoreInt32(&restartInProgress, 0)) and return the error. Ensure you reference the same symbols: restartServer, restartInProgress, shutdownServer, os.Executable, exec.Command, and cmd.Start.
🧹 Nitpick comments (2)
web/src/components/layout/app-sidebar/nav-user.tsx (2)
42-45: 类型断言重复定义
desktopRestartAvailable的类型断言(Lines 42-45)和handleRestartServer内部的类型断言(Lines 55-57)略有不同:前者返回unknown,后者返回Promise<void>。建议统一类型定义以提高可维护性。♻️ 建议的重构方案
+type DesktopLauncher = { + go?: { desktop?: { LauncherApp?: { RestartServer?: () => Promise<void> } } }; +}; const desktopRestartAvailable = typeof window !== 'undefined' && - !!(window as unknown as { go?: { desktop?: { LauncherApp?: { RestartServer?: () => unknown } } } }) + !!(window as unknown as DesktopLauncher) .go?.desktop?.LauncherApp?.RestartServer; const handleRestartServer = async () => { if (!window.confirm(t('nav.restartServerConfirm'))) return; try { if (desktopRestartAvailable) { - const launcher = (window as unknown as { - go?: { desktop?: { LauncherApp?: { RestartServer?: () => Promise<void> } } }; - }).go?.desktop?.LauncherApp; + const launcher = (window as unknown as DesktopLauncher).go?.desktop?.LauncherApp;Also applies to: 54-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/app-sidebar/nav-user.tsx` around lines 42 - 45, The two type assertions are inconsistent: unify the signature used for the LauncherApp.RestartServer across desktopRestartAvailable and handleRestartServer by introducing a single shared type/interface (e.g., an extended Window type or LauncherApp interface) where go.desktop.LauncherApp.RestartServer is declared as () => Promise<void>, then replace both inline casts with that shared type (use it in the desktopRestartAvailable check and the call site in handleRestartServer) so the type is consistent and maintainable.
226-232: Fragment 包装器可移除此处的空 Fragment (
<>...</>) 包裹DropdownMenuSeparator和DropdownMenuItem是多余的,因为它们可以直接作为DropdownMenuContent的子元素。♻️ 建议的简化方案
- <> - <DropdownMenuSeparator /> - <DropdownMenuItem onClick={handleRestartServer}> - <RefreshCw /> - <span>{t('nav.restartServer')}</span> - </DropdownMenuItem> - </> + <DropdownMenuSeparator /> + <DropdownMenuItem onClick={handleRestartServer}> + <RefreshCw /> + <span>{t('nav.restartServer')}</span> + </DropdownMenuItem>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/layout/app-sidebar/nav-user.tsx` around lines 226 - 232, 当前在 DropdownMenuContent 内使用了多余的空 Fragment 包裹 DropdownMenuSeparator 和 DropdownMenuItem(包含 handleRestartServer 调用),请直接移除该 Fragment,使 DropdownMenuSeparator 和 DropdownMenuItem 成为 DropdownMenuContent 的直接子元素(保留 RefreshCw 图标和 handleRestartServer 处理器及翻译键 t('nav.restartServer') 不变)。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cmd/maxx/main.go`:
- Around line 451-475: The restartServer closure currently calls
shutdownServer("restart") before confirming that os.Executable() and starting
the new process (exec.Command + cmd.Start()) succeed; if those fail the server
is down with restartInProgress set. Change restartServer so it first obtains and
validates the executable path (os.Executable), constructs the exec.Command
(os.Args[1:], os.Environ()), and attempts to start the new process (cmd.Start())
— only after cmd.Start() succeeds should shutdownServer("restart") be invoked
and os.Exit called; on any failure, ensure restartInProgress is reset
(atomic.StoreInt32(&restartInProgress, 0)) and return the error. Ensure you
reference the same symbols: restartServer, restartInProgress, shutdownServer,
os.Executable, exec.Command, and cmd.Start.
---
Nitpick comments:
In `@web/src/components/layout/app-sidebar/nav-user.tsx`:
- Around line 42-45: The two type assertions are inconsistent: unify the
signature used for the LauncherApp.RestartServer across desktopRestartAvailable
and handleRestartServer by introducing a single shared type/interface (e.g., an
extended Window type or LauncherApp interface) where
go.desktop.LauncherApp.RestartServer is declared as () => Promise<void>, then
replace both inline casts with that shared type (use it in the
desktopRestartAvailable check and the call site in handleRestartServer) so the
type is consistent and maintainable.
- Around line 226-232: 当前在 DropdownMenuContent 内使用了多余的空 Fragment 包裹
DropdownMenuSeparator 和 DropdownMenuItem(包含 handleRestartServer 调用),请直接移除该
Fragment,使 DropdownMenuSeparator 和 DropdownMenuItem 成为 DropdownMenuContent
的直接子元素(保留 RefreshCw 图标和 handleRestartServer 处理器及翻译键 t('nav.restartServer') 不变)。
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cmd/maxx/main.gointernal/desktop/launcher.gointernal/handler/admin.goweb/src/components/layout/app-sidebar/nav-user.tsxweb/src/lib/transport/http-transport.tsweb/src/lib/transport/interface.tsweb/src/locales/en.jsonweb/src/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (4)
- web/src/lib/transport/interface.ts
- web/src/locales/zh.json
- web/src/locales/en.json
- web/src/lib/transport/http-transport.ts
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (2)
cmd/maxx/main.go (1)
internal/core/server.go (2)
GracefulShutdownTimeout(16-16)HTTPShutdownTimeout(18-18)
web/src/components/layout/app-sidebar/nav-user.tsx (3)
web/src/components/theme-provider.tsx (1)
useTheme(138-144)launcher/script.js (1)
launcher(22-31)web/src/components/ui/dropdown-menu.tsx (2)
DropdownMenuSeparator(239-239)DropdownMenuItem(235-235)
🔇 Additional comments (6)
web/src/components/layout/app-sidebar/nav-user.tsx (1)
51-71: 重启逻辑实现合理
handleRestartServer函数实现了以下功能:
- 用户确认对话框
- 桌面端优先使用原生重启,否则回退到 HTTP transport
- 完善的错误处理和用户反馈
实现清晰,逻辑正确。
internal/handler/admin.go (2)
110-129: 重启端点实现正确
handleRestart实现合理:
- 强制 POST 方法
- 未配置重启回调时返回 501 Not Implemented
- 异步执行重启并立即返回 202 Accepted
异步 fire-and-forget 模式适合重启场景,因为服务器即将关闭,无法等待结果。错误仅记录日志是合理的设计选择。
36-39: SetRestartFunc 方法实现简洁简单的 setter 方法,允许外部组件注入重启回调,符合依赖注入原则。
internal/desktop/launcher.go (1)
209-212: 桌面端重启回调正确绑定在组件初始化后将
RestartServer绑定到AdminHandler,使 Web 管理端点能够触发桌面端重启。空值检查确保安全。桌面版采用进程内重启(重新初始化组件),与 CLI 版本的进程替换方式不同,两种方式都适合各自的运行环境。
cmd/maxx/main.go (2)
406-449: 优雅关闭实现完善
shutdownServer函数实现了完整的优雅关闭流程:
- 等待活跃请求完成(使用
GracefulShutdownTimeout)- 停止后台清理任务
- 停止 pprof manager
- 停止 Codex OAuth 服务器
- 关闭 HTTP 服务器
逻辑清晰,超时处理合理。
477-477: 重启回调正确绑定到 AdminHandler将
restartServer函数绑定到adminHandler,使 Web 管理端点能够触发服务器重启。由于adminHandler受authMiddleware保护(Line 355),此功能具有适当的访问控制。
Summary
Testing
Summary by CodeRabbit
发布说明